Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SR] Support replays for crashes in buffer and session modes #3609

Merged
merged 10 commits into from
Jul 30, 2024

Conversation

romtsn
Copy link
Member

@romtsn romtsn commented Jul 29, 2024

#skip-changelog

📜 Description

  • Make buffer mode work for ANRs
    • For that we have to store errorSampleRate to disk and then roll the dice on next launch, because it could change in the meantime
    • Also, since in buffer mode we don't actually write replayId to scope until an error happens (and in the case of an ANR we don't know until next restart), the persisted replayId to disk might be irrelevant. So we have to iterate through the leftover replay folders and find the one that was last modified closest to the ANR and take it as the previous replay
    • Important part is: we set the relevant replayId back to the disk cache, so later ReplayIntegration can pick it up and finalize and send it. The order will be ensured by an integration test
  • Make buffer mode work for Crashes
    • Renamed sendReplayForEvent to captureReplay which just accept one argument now to define whether the app process is terminating or not. If it is, then we don't capture the buffered replay in the current process, but will capture it on next launch, to avoid weird issues with half-captured videos
    • Refactored createSegment method and moved it to a static method under CaptureStrategy so we could call it from ReplayIntegration as well
  • Small fix for network breadcrumbs where we could deserialize double values sometimes (didn't want to change the deserializer for that to not break something accidentally)
  • Small fix for the video encoder, set IFRAME_INTERVAL to -1 so it always only encode partial updates without full frames to save the video size

💡 Motivation and Context

Part of getsentry/sentry#74441

💚 How did you test it?

manually + automated

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

Copy link
Contributor

github-actions bot commented Jul 29, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 425.89 ms 494.59 ms 68.70 ms
Size 1.70 MiB 2.35 MiB 659.95 KiB

Baseline results on branch: rz/feat/session-replay-anrs

Startup times

Revision Plain With Sentry Diff
c08178b 463.49 ms 521.56 ms 58.07 ms
67eeea2 418.83 ms 468.14 ms 49.31 ms
cde588a 434.92 ms 521.49 ms 86.57 ms
1a9eb03 380.06 ms 449.65 ms 69.59 ms
80061b7 395.55 ms 462.64 ms 67.09 ms
11e9a81 384.38 ms 469.45 ms 85.07 ms
eb8377e 325.29 ms 378.86 ms 53.57 ms

App size

Revision Plain With Sentry Diff
c08178b 1.70 MiB 2.34 MiB 657.87 KiB
67eeea2 1.70 MiB 2.34 MiB 657.87 KiB
cde588a 1.70 MiB 2.34 MiB 658.00 KiB
1a9eb03 1.70 MiB 2.34 MiB 657.61 KiB
80061b7 1.70 MiB 2.34 MiB 658.13 KiB
11e9a81 1.70 MiB 2.34 MiB 658.20 KiB
eb8377e 1.70 MiB 2.34 MiB 658.10 KiB

Previous results on branch: rz/feat/session-replay-crashes

Startup times

Revision Plain With Sentry Diff
f92f141 409.55 ms 490.19 ms 80.65 ms
828b30b 446.63 ms 539.67 ms 93.04 ms
2e2a18e 543.10 ms 617.62 ms 74.52 ms

App size

Revision Plain With Sentry Diff
f92f141 1.70 MiB 2.34 MiB 659.63 KiB
828b30b 1.70 MiB 2.35 MiB 660.25 KiB
2e2a18e 1.70 MiB 2.34 MiB 659.62 KiB

@romtsn romtsn marked this pull request as ready for review July 30, 2024 07:33
Copy link
Member

@markushi markushi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one, great to see this all coming together. Left a few minor comments

@romtsn romtsn merged commit 67fede6 into rz/feat/session-replay-anrs Jul 30, 2024
22 checks passed
@romtsn romtsn deleted the rz/feat/session-replay-crashes branch July 30, 2024 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants